Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

makefiles/tools/jlink.inc.mk: use ELF file for flashing #19541

Merged
merged 3 commits into from
May 4, 2023
Merged

makefiles/tools/jlink.inc.mk: use ELF file for flashing #19541

merged 3 commits into from
May 4, 2023

Conversation

maribu
Copy link
Member

@maribu maribu commented May 3, 2023

Contribution description

There seems to be a consensus to go for a more ELF centric approach whenever possible. J-Link seems to work fine with ELF files, so let's avoid the extra step to create a bin file from the ELF file prior to flashing.

Testing procedure

Flashing with J-Link should still just work

Issues/PRs references

In #19537 (comment) I was made aware that we use the bin file by default for J-Link flashing.

@github-actions github-actions bot added Area: build system Area: Build system Area: tools Area: Supplementary tools labels May 3, 2023
@maribu maribu mentioned this pull request May 3, 2023
Merged
@dylad
Copy link
Member

dylad commented May 3, 2023

I am wondering if we should also update this file at the same time.
Since we used .bin file, we had to manually prepare the offset address to flash. But I don't know if it will have side effects, to specify an offset AND passing the .elf at the same time. the latter is supposed to contain this information already but I don't know if jlink command will ignore the offset argument then.
I'll run some tests tonight if I can.

@maribu
Copy link
Member Author

maribu commented May 3, 2023

Indeed, that shell script should be adapted to handle ELF files properly

@maribu maribu force-pushed the tools/jlink/elffile branch from f041197 to 6dd88e2 Compare May 3, 2023 13:30
@maribu
Copy link
Member Author

maribu commented May 3, 2023

I updated the script to handle both ELF and bin files. I tested both successfully with the nRF52840DK.

maribu added 3 commits May 3, 2023 15:32
@maribu
dist/tools/jlink/jlink.sh: allow flashing with ELF file
666dc23 
Adapt the script to flash files as ELF file if the file extension is
`.elf`, otherwise stick with default behavior and flash as bin file.
@maribu
makefiles/tools/jlink.inc.mk: use ELF file for flashing
63d535e 
There seems to be a consensus to go for a more ELF centric approach
whenever possible. J-Link seems to work fine with ELF files, so let's
avoid the extra step to create a bin file from the ELF file prior to
flashing.
@maribu
boards/rpi-pico: ELF format now is default, cleanup
93561c7 
Now that the ELF file format is the default anyway, we don't need to
specify this explicitly anymore.
@maribu maribu force-pushed the tools/jlink/elffile branch from 6dd88e2 to 93561c7 Compare May 3, 2023 13:33
@github-actions github-actions bot added the Area: boards Area: Board ports label May 3, 2023
@maribu
Copy link
Member Author

maribu commented May 3, 2023

Rebased on master and dropped the now no longer needed FLASHFILE = $(ELFFILE).

dylad
dylad approved these changes May 3, 2023
View reviewed changes
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this PR on rpi-pico both with .elf file and .bin file (with IMAGE_OFFSET=0x10000000) and it works as expected.

@dylad dylad added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 3, 2023
@riot-ci
Copy link

riot-ci commented May 3, 2023

Murdock results

✔️ PASSED

93561c7 boards/rpi-pico: ELF format now is default, cleanup

Success Failures Total Runtime
6931 0 6931 10m:29s

Artifacts

@dylad
Copy link
Member

dylad commented May 3, 2023

bors merge

bors bot added a commit that referenced this pull request May 3, 2023
@bors @maribu
Merge #19541
e0f9c51 
19541: makefiles/tools/jlink.inc.mk: use ELF file for flashing r=dylad a=maribu

### Contribution description

There seems to be a consensus to go for a more ELF centric approach whenever possible. J-Link seems to work fine with ELF files, so let's avoid the extra step to create a bin file from the ELF file prior to flashing.


Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
@bors
Copy link
Contributor

bors bot commented May 3, 2023

Build failed:

@maribu
Copy link
Member Author

maribu commented May 4, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented May 4, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit ae51cbc into RIOT-OS:master May 4, 2023
@maribu maribu deleted the tools/jlink/elffile branch May 4, 2023 06:57
@maribu
Copy link
Member Author

maribu commented May 4, 2023

Thx :)

@benpicco
Copy link
Contributor

benpicco commented Jul 7, 2023

This breaks flashing nrf52840dk:

### Flashing Target ###
### Flashing elf file ###
SEGGER J-Link Commander V7.66b (Compiled May 30 2022 14:12:34)
DLL version V7.66b, compiled May 30 2022 14:12:19

J-Link Commander will now exit on Error

J-Link Command File read successfully.
Processing script file...
J-Link>loadfile /home/benpicco/dev/RIOT/tests/sys/vfs_default/bin/nrf52840dk/tests_vfs_default.elf
J-Link connection not established yet but required for command.
Connecting to J-Link via USB...O.K.
Firmware: J-Link OB-SAM3U128-V2-NordicSemi compiled Jan 20 2022 16:43:31
Hardware version: V1.00
S/N: 683332925
License(s): RDI, FlashBP, FlashDL, JFlash, GDB
USB speed mode: High speed (480 MBit/s)
VTref=3.300V
Target connection not established yet but required for command.
Device "NRF52" selected.


Connecting to target via SWD
InitTarget() start
InitTarget() end
Found SW-DP with ID 0x2BA01477
DPIDR: 0x2BA01477
CoreSight SoC-400 or earlier
Scanning AP map to find all available APs
AP[2]: Stopped AP scan as end of AP map has been reached
AP[0]: AHB-AP (IDR: 0x24770011)
AP[1]: JTAG-AP (IDR: 0x02880000)
Iterating through AP map to find AHB-AP to use
AP[0]: Core found
AP[0]: AHB-AP ROM base: 0xE00FF000
CPUID register: 0x410FC241. Implementer code: 0x41 (ARM)
Found Cortex-M4 r0p1, Little endian.
FPUnit: 6 code (BP) slots and 2 literal slots
CoreSight components:
ROMTbl[0] @ E00FF000
[0][0]: E000E000 CID B105E00D PID 000BB00C SCS-M7
[0][1]: E0001000 CID B105E00D PID 003BB002 DWT
[0][2]: E0002000 CID B105E00D PID 002BB003 FPB
[0][3]: E0000000 CID B105E00D PID 003BB001 ITM
[0][4]: E0040000 CID B105900D PID 000BB9A1 TPIU
[0][5]: E0041000 CID B105900D PID 000BB925 ETM
Cortex-M4 identified.
Downloading file [/home/benpicco/dev/RIOT/tests/sys/vfs_default/bin/nrf52840dk/tests_vfs_default.elf]...
File is of unknown / supported format.

Script processing completed.

@benpicco
Copy link
Contributor

benpicco commented Jul 7, 2023

Hm nvm, updating JLinkExe to 7.8811 also fixed this.

@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@dylad dylad dylad approved these changes

@chrysn chrysn Awaiting requested review from chrysn

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@kaspar030 kaspar030 Awaiting requested review from kaspar030

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

Assignees
No one assigned
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Milestone
Release 2023.07
Development

Successfully merging this pull request may close these issues.
4 participants
@maribu @dylad @riot-ci @benpicco